Skip to content

fix: apply follow-up quality fixes#8

Closed
ZeR020 wants to merge 1 commit into
mainfrom
audit-full-repo-3
Closed

fix: apply follow-up quality fixes#8
ZeR020 wants to merge 1 commit into
mainfrom
audit-full-repo-3

Conversation

@ZeR020

@ZeR020 ZeR020 commented May 6, 2026

Copy link
Copy Markdown
Owner

Request

@coderabbitai Please perform a comprehensive full-repo audit of the entire codebase. This PR touches all 83 source, test, and script files to ensure every module is reviewed.

Review Focus

Please thoroughly review all 83 changed files with emphasis on:

  1. Security — SQL injection vectors, path traversal, secret exposure, unsafe eval, missing input sanitization
  2. Type Safety — any any types that could be narrowed, missing null checks, unsafe casts
  3. Error Handling — swallowed errors, uncaught async rejections, missing rollback logic
  4. Resource Leaks — unclosed DB connections, file handles, event listeners, timers
  5. Logic Bugs — race conditions, off-by-one errors, incorrect async sequencing, state mutations
  6. Cross-Platform Issues — Bun/Node.js runtime differences, Windows path handling
  7. Test Coverage — missing edge cases, flaky tests, mocked vs real behavior gaps
  8. Performance — N+1 queries, unnecessary allocations, blocking operations in async paths

Context

  • This is a fresh audit after previous rounds of critical and major fixes were applied
  • The codebase is a persistent-memory plugin for OpenCode with:
    • SQLite + vector search backend
    • Bun/Node.js dual runtime support
    • Web UI + REST API
    • LLM-powered conflict resolution
    • Multi-factor memory scoring

Do Not Merge

This PR is for audit purposes only. No behavioral changes are introduced — only non-functional audit trigger comments.

Summary by CodeRabbit

  • Chores
    • Added internal audit tracking instrumentation across the codebase for maintenance purposes.

Add non-functional audit markers to 83 files across src/, tests/, and scripts/
to trigger a comprehensive CodeRabbit review of the entire repository.

No behavioral, API, or logic changes are introduced.
Copilot AI review requested due to automatic review settings May 6, 2026 16:46
@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR systematically appends audit trigger comment markers across the entire repository to enable tracking during a Round 3 full-repository audit. Approximately 60 files receive trailing comments, though several files also contain non-functional numeric line artifacts that warrant verification.

Changes

Repository-Wide Audit Trigger Markers

Layer / File(s) Summary
Root & Build Scripts
scripts/build.mjs, scripts/migrate-tests.mjs, scripts/migrate-v1-to-v2.ts
Audit marker comments appended to build and migration scripts.
Core Configuration & Exports
src/config.ts, src/index.ts, src/plugin.ts, src/types/index.ts, src/types/usearch.d.ts
Audit markers added to top-level module configuration and type definitions.
AI Service Layer
src/services/ai/ai-provider-factory.ts, src/services/ai/opencode-provider.ts, src/services/ai/provider-config.ts, src/services/ai/providers/*
Audit markers appended to AI provider implementations and factory patterns.
Session & Schema Management
src/services/ai/session/ai-session-manager.ts, src/services/ai/session/session-types.ts, src/services/ai/tools/tool-schema.ts, src/services/ai/validators/user-profile-validator.ts
Audit markers added to session and validation infrastructure.
Core Services
src/services/api-handlers.ts, src/services/auto-capture.ts, src/services/cleanup-service.ts, src/services/client.ts, src/services/context.ts, src/services/deduplication-service.ts, src/services/embedding.ts, src/services/jsonc.ts, src/services/language-detector.ts, src/services/logger.ts
Audit markers added to general utility and handler services.
Memory Management Services
src/services/memory-conflicts.ts, src/services/memory-lifecycle.ts⚠️, src/services/memory-scoring-service.ts, src/services/memory-scoring.ts⚠️, src/services/migration-service.ts
Audit markers appended; some files contain suspicious numeric line artifacts (404–406, 619–621) requiring verification.
Infrastructure Services
src/services/platform-server.ts, src/services/privacy.ts, src/services/retrieval-context.ts⚠️, src/services/secret-resolver.ts
Audit markers added; retrieval-context.ts includes stray numeric tokens (163–165).
SQLite & Vector Backend Services
src/services/sqlite/..., src/services/vector-backends/...
Audit markers appended to database and vector search infrastructure; includes connection-manager.ts, shard-manager.ts, sqlite-bootstrap.ts, transcript-manager.ts, types.ts, vector-search.ts, backend-factory.ts, exact-scan-backend.ts, types.ts, usearch-backend.ts.
User Profile & Prompt Services
src/services/user-profile/..., src/services/user-prompt/user-prompt-manager.ts, src/services/user-memory-learning.ts, src/services/tags.ts, src/services/transcript-capture.ts
Audit markers added to user-facing data and interaction services.
Web Server Services
src/services/web-server-worker.ts⚠️, src/services/web-server.ts⚠️
Audit markers appended; both files contain stray numeric literals (367–368, 523–525) within function bodies that break code syntax.
Web Frontend
src/web/app.js, src/web/i18n.js
Audit markers added to frontend application files.
Test Suite
tests/*.test.ts, tests/vector-backends/*.test.ts
Audit markers appended to test files; tests/tool-scope.test.ts⚠️ contains stray numeric lines (157–159).

⚠️ Attention Required: Files marked with ⚠️ contain non-functional numeric line artifacts that appear to be unintended syntax artifacts. Verify whether these are intentional audit instrumentation or merge/generation errors before merging.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

The changes are largely repetitive (the same comment pattern across ~60 files), which reduces complexity, but the presence of syntax-breaking numeric artifacts in 5 key service files (memory-lifecycle.ts, memory-scoring.ts, web-server-worker.ts, web-server.ts, tool-scope.test.ts) demands careful verification to ensure they are intentional instrumentation rather than accidental insertions.

Poem

🐰 Hopping through the code so wide,
Audit triggers placed with pride,
Round Three comes to every nook,
But those stray numbers—take a look!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 32.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title '🔍 Round 3 — Full Codebase Audit' is vague and non-descriptive, relying on emoji and generic terms without clearly conveying the actual changeset purpose. Replace with a specific, descriptive title like 'Add audit trigger markers across codebase for Round 3 review' that clearly indicates non-functional audit marker additions.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch audit-full-repo-3

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/services/ai/tools/tool-schema.ts (1)

8-9: ⚡ Quick win

Narrow any in schema properties to improve tool payload safety.

Lines 8, 20, and 30 use Record<string, any>, which weakens compile-time checks for tool schemas.

Suggested refactor
-      properties: Record<string, any>;
+      properties: Record<string, unknown>;

Also applies to: 20-21, 30-31

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/ai/tools/tool-schema.ts` around lines 8 - 9, The schema's
properties field currently uses Record<string, any> (appearing on the properties
declarations around lines with properties: Record<string, any>), which weakens
type safety; change those occurrences to a safer type such as Record<string,
unknown> or introduce a generic type parameter (e.g., ToolSchema<T = unknown> or
a named SchemaProperties = Record<string, unknown>) and apply it to the
properties declarations so callers can narrow payload shapes; update the
interface/type declarations that reference properties (and any functions or
classes using ToolSchema) to use the new generic or SchemaProperties alias to
preserve compatibility and stronger compile-time checks.
src/services/api-handlers.ts (1)

511-536: 🏗️ Heavy lift

Untagged search does repeated shard scans (N×M scaling).

For each distinct tag, the code fetches shards and searches again, which can explode query cost as tags grow.

✅ Suggested direction
-      for (const containerTag of uniqueTags) {
-        const { scope, hash } = extractScopeFromTag(containerTag);
-        const shards = shardManager.getAllShards(scope, hash);
-        for (const shard of shards) {
-          ...
-        }
-      }
+      // Build a deduplicated shard set first, then search each shard once
+      // with broader filtering and merge results centrally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/api-handlers.ts` around lines 511 - 536, The current loop does
repeated shard scans by iterating uniqueTags and calling
shardManager.getAllShards + vectorSearch.searchInShard for each tag; instead
build a mapping from shard (use shard.id or shard.dbPath from
projectShards/shardManager.getAllShards) to the list of container tags that
belong to that shard (use extractScopeFromTag to determine scope/hash), then
iterate each shard once and perform a single search per shard (modify or
overload vectorSearch.searchInShard to accept multiple container tags or run one
search and filter by tags in-memory) and push results into memoryResults; update
error handling around the single shard search accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@scripts/migrate-tests.mjs`:
- Around line 30-31: The regex used in the content.replace call (\bmock\() also
matches the `mock(` part of existing `vi.mock(` calls causing `vi.vi.fn(`;
update the replacement regex to skip instances already prefixed by `vi.` (e.g.
use a negative lookbehind like `(?<!\bvi\.)mock\(`) so only standalone `mock(`
becomes `vi.fn(`, and keep the replacement logic that uses content.replace to
perform the rewrite.

In `@scripts/migrate-v1-to-v2.ts`:
- Around line 166-225: The createTranscriptsDb function currently opens a
Database (via new Database(...)) but only calls db.close() on the success path,
leaking the DB handle on exceptions; fix by ensuring the Database instance is
closed in all cases (use a try/finally or similar): declare a db variable before
the try, assign it from new Database(...) inside the try, and call db.close() in
a finally block (guarding for undefined) so the DB is closed on success or error
while retaining the existing logging behavior in the catch; references:
createTranscriptsDb, Database, db.close.
- Around line 306-344: The per-database Database(dbPath) handle can leak if any
of isV1Schema, addV2Columns, backfillScores, createConflictsTable, or
addV2Indexes throws; wrap the per-db work in a try/finally so the Database
instance is always closed (e.g., after constructing const db = new
Database(dbPath) do try { ...normal migration steps... } finally { if (db)
db.close(); }) and keep the existing catch to record errors (or move the catch
inside the try so finally still runs); ensure PRAGMA wal_checkpoint(TRUNCATE)
runs before closing where appropriate but still call db.close() in the finally
block to avoid SQLite lock leaks.

In `@src/config.ts`:
- Around line 237-245: In loadConfigFromPaths, do not silently swallow errors
when reading/parsing files; when existsSync(path) is true but readFileSync,
stripJsoncComments, or JSON.parse fails, catch the error and rethrow or surface
it (e.g., throw new Error including the path and original error.message) instead
of an empty catch block so misconfigured JSONC is visible; update the catch
around readFileSync/stripJsoncComments/JSON.parse to include the path and
original error (or log via your logger) and then rethrow to stop silent fallback
to defaults.
- Line 560: Update the file creation call that writes CONFIG_TEMPLATE to ensure
owner-only permissions: when calling writeFileSync with configPath and
CONFIG_TEMPLATE, pass an options object (instead of the string "utf-8") that
sets encoding to "utf-8" and mode to 0o600 so the created config file is
readable/writable only by the owner; reference the existing writeFileSync
invocation and the CONFIG_TEMPLATE and configPath identifiers to locate and
change the call accordingly.

In `@src/index.ts`:
- Around line 520-547: The current handler uses a single global idleTimeout
which causes one session's idle timer to cancel another; change to a per-session
timer map (e.g., sessionIdleTimers: Map<string, Timeout>) inside the module and
in the event: async (...) block use sessionID to clear/set the timer for that
session only (clear existing sessionIdleTimers.get(sessionID), set a new timeout
that calls performAutoCapture(ctx, sessionID, directory) and, on finally,
deletes the entry from sessionIdleTimers). Keep the existing post-capture work
(performUserProfileLearning, cleanupService.shouldRunCleanup/runCleanup,
cleanupOldTranscripts, connectionManager.checkpointAll) inside the per-session
timeout callback so each session’s idle handling is independent.

In `@src/services/ai/opencode-provider.ts`:
- Around line 115-123: The try/catch blocks around reading/writing auth.json
silently swallow errors; update the code around readFileSync(authPath),
JSON.parse(...), writeFileSync(authPath, ...), and chmodSync(authPath, ...) to
catch errors and log them (include providerName and authPath in the message)
instead of empty catches—use the module's existing logger if available (e.g.,
logger.error) or console.error as a fallback so failures to read or persist Auth
for providerName are visible for debugging.
- Around line 280-288: The code directly reads result.output from the
generateText call in the function returning as T (the block using generateText
with model: provider(options.modelId), output: Output.object({ schema:
options.schema })) which can throw NoOutputGeneratedError when
parsing/finishReason fails; wrap the generateText call in a try/catch, catch
NoOutputGeneratedError (and other errors), and handle it appropriately (e.g.,
throw a more descriptive error, return a safe default, or propagate a typed
error) instead of blindly casting result.output as T so callers do not receive
an unhandled exception.

In `@src/services/ai/providers/anthropic-messages.ts`:
- Line 213: The timeout error message is using this.config.iterationTimeout
(which can be undefined) instead of the resolved local iterationTimeout, so
change the error text in the AnthropicMessages provider to reference the
resolved iterationTimeout variable used earlier in the method (e.g., replace
`this.config.iterationTimeout` with `iterationTimeout` in the `error: \`API
request timeout (...)\`` message), ensuring `iterationTimeout` is the same
computed value (defaulted if necessary) used for the actual timeout logic.
- Around line 77-83: The current two-step sequence assignment using
aiSessionManager.getLastSequence(...) + 1 followed by
aiSessionManager.addMessage(...) is race-prone under concurrent requests; change
AISessionManager to provide an atomic sequence allocator method (e.g.,
allocateNextSequence(sessionId) or addMessageWithSequence(sessionId, message))
that computes and persists the next sequence in one operation, then replace all
call sites that use getLastSequence + addMessage (including the user sequence
block and the assistant/retry write sites referenced around methods
getLastSequence and addMessage) to use the new atomic method so sequence
allocation and message insert are performed together.

In `@src/services/ai/providers/base-provider.ts`:
- Around line 30-38: applySafeExtraParams currently copies keys from extraParams
into requestBody but doesn't guard against prototype-polluting names; update
applySafeExtraParams to skip any keys that are prototype attack vectors (at
minimum "__proto__", "constructor", and "prototype") in addition to existing
PROTECTED_KEYS, and ensure you only assign when the key is an own property (use
Object.prototype.hasOwnProperty.call(extraParams, key) or rely on Object.entries
as now) before setting requestBody[key] = value; reference function
applySafeExtraParams and PROTECTED_KEYS and add the explicit checks for
"__proto__", "constructor", and "prototype" to prevent prototype pollution.

In `@src/services/ai/providers/google-gemini.ts`:
- Around line 153-155: The current code appends the Google API key to the query
string (building url via baseUrl and url variables) which exposes credentials;
instead remove the "?key=..." query parameter from the URL and send the API key
in the HTTP header "x-goog-api-key" when making the request (use
this.config.apiKey). Update the request invocation in the same method that
constructs baseUrl/url so the URL becomes
`${baseUrl}/models/${this.config.model}:generateContent` and ensure the request
headers include { "x-goog-api-key": this.config.apiKey } (retain any existing
headers like Content-Type/Authorization).

In `@src/services/ai/providers/openai-chat-completion.ts`:
- Around line 371-381: The current log call in openai-chat-completion.ts that
invokes log("OpenAI tool response validation failed", ...) must not persist raw
tool arguments; replace the rawArguments entry
(toolCall.function.arguments.slice(0, 500)) with non-sensitive metadata such as
the argument size/length and a deterministic hash (e.g., SHA256) or a redaction
flag, and/or an error code; update the log payload in the log(...) call that
references toolSchema.function.name and toolCall.function.arguments so it emits
only metadata (length/hash/redacted) rather than the argument text itself.

In `@src/services/ai/session/ai-session-manager.ts`:
- Around line 23-31: The AISessionManager is being constructed eagerly at import
time and dereferences CONFIG.storagePath, causing startup failures when CONFIG
is not yet populated; fix by making AISessionManager lazily initialized: remove
any top-level/new AISessionManager() and add a static getInstance() that
constructs the manager on first call, and in the constructor defer reading
CONFIG.storagePath and DB setup (move db/open logic and initDatabase() out of
the constructor into a private initIfNeeded() invoked on first use), and apply
the same lazy-init change to the other occurrence referenced (the second eager
construction at the other location).
- Around line 156-191: The current addMessage and getLastSequence in
AISessionManager are racy because writers can read the same last sequence and
insert duplicate sequence numbers; replace them with a single atomic operation
(e.g., new method addMessageAtomic or insertNextSequenceMessage) that computes
the next sequence and inserts in one DB operation or inside a DB transaction.
Implement this by using the ai_messages table in a transaction or an INSERT ...
SELECT that computes COALESCE(MAX(sequence), -1)+1 for the given ai_session_id
and then performs the INSERT with that computed sequence, and update callers to
use the new addMessageAtomic instead of calling getLastSequence + addMessage.

In `@src/services/api-handlers.ts`:
- Around line 1091-1097: handleListConflicts ignores its resolved parameter
because it always calls getAllUnresolvedConflicts(limit); update
handleListConflicts to respect the resolved boolean by calling the appropriate
function (e.g., getAllResolvedConflicts(limit) when resolved is true) or by
calling a generic getAllConflicts(limit) and filtering by conflict.resolved
before mapping, and ensure the returned array and any pagination/limit logic
apply to the chosen set; modify the call site inside handleListConflicts to use
the resolved-aware source instead of always calling getAllUnresolvedConflicts.
- Around line 411-443: The update flow currently deletes the existing vector
before computing embeddings and reinserting, which can permanently lose data if
embedding/insert fails; in handleUpdateMemory change the sequence to compute
embeddings (embeddingService.embedWithTimeout) and build updatedRecord first,
then attempt to insert the new vector via vectorSearch.insertVector into the
target shard, only after a successful insert call delete the old vector with
vectorSearch.deleteVector and update shard counts (call
shardManager.incrementVectorCount for the new insert and
shardManager.decrementVectorCount for the old delete); additionally add failure
handling to roll back a successful insert (delete the newly inserted vector and
adjust counts) if the subsequent delete fails, or surface an error without
deleting the original if insert fails—update code around
deleteVector/insertVector and
shardManager.incrementVectorCount/decrementVectorCount to follow this
transactional sequence.

In `@src/services/cleanup-service.ts`:
- Line 65: linkedMemoriesDeleted is initialized but never incremented, so
callers always see 0; update the cleanup logic to increment
linkedMemoriesDeleted whenever a memory from linkedMemoryIds is actually deleted
(where deletions occur in the cleanup loop that removes memories/prompts), e.g.,
after the code path that calls the memory deletion routine (refer to
linkedMemoryIds and the memory deletion function in CleanupResult-producing
code), and ensure the final returned CleanupResult carries that updated count;
alternatively, if you intend to skip deleting protected linked memories, update
the logic or the CleanupResult interface to reflect that no linked deletions
occur.

In `@src/services/deduplication-service.ts`:
- Around line 89-106: Guard against malformed vector blobs by validating
mem1.vector and mem2.vector byte-length/alignment before constructing
Float32Array: check that the vector exists, is a
Buffer/Uint8Array/ArrayBuffer-compatible object and that its byteLength % 4 ===
0; if invalid, log a warning (including the memory id and container_tag) and
skip that memory (mark processedIds or continue) so the deduplication loop in
the block that creates vector1 and vector2 (referencing mem1.vector,
mem2.vector, uniqueMemories, processedIds, and DuplicateGroup) does not throw;
preserve current flow to only call this.cosineSimilarity when both vectors pass
validation.

In `@src/services/embedding.ts`:
- Around line 105-107: In the embedding creation block (where response, data,
result and Float32Array are used), validate the external API payload before
indexing into data.data[0].embedding: check that data exists, data.data is an
array, data.data[0] exists, and data.data[0].embedding is an array of numbers;
if validation fails, throw or return a clear error instead of dereferencing the
path. Convert/guard elements to numbers when constructing the Float32Array
(e.g., map to Number) and surface a descriptive error mentioning the response
shape and the raw payload for debugging.
- Around line 24-29: The withTimeout function (withTimeout) leaks timers because
the timeout created by setTimeout is never cleared; update withTimeout to store
the timeout id (e.g., let timer) and clear it (clearTimeout(timer)) when either
the main promise resolves/rejects or when the timeout fires, and ensure the
Promise.race helper uses a timeout promise that calls reject only after
setting/clearing appropriately so the timer is cleaned up in both resolution and
rejection paths.

In `@src/services/logger.ts`:
- Around line 56-63: The log function can throw when JSON.stringify(data)
encounters circular refs or BigInt; update log (and its helpers if any) to
safely serialize the optional data without throwing: in log(), wrap the
serialization in a try/catch and attempt a safe fallback (e.g., convert BigInt
to string and use util.inspect or a circular-safe replacer) so that if
JSON.stringify fails you still produce a string representation rather than
throwing; ensure appendFileSync is always called with a constructed line and
never propagates serializer exceptions from log(), referencing the log,
ensureLoggerInitialized, and getLogFilePath symbols to locate the code to
change.

In `@src/services/memory-conflicts.ts`:
- Around line 291-303: The FTS MATCH query built in the words -> query pipeline
can be manipulated by FTS metacharacters (e.g., *, ^, NEAR, AND/OR/NOT), so
sanitize the search tokens before constructing query: update the words array
processing (the code that currently uses words.map and builds query) to
normalize each token by removing or escaping FTS operators (e.g., strip
characters like *, ^, :, -, +, ?, parentheses and the reserved words
NEAR/AND/OR/NOT) or replace them with safe alphanumerics/space using a whitelist
regex, then re-apply the existing double-quote wrapping; keep using the same
query variable and pass it to db.prepare(... WHERE memories_fts MATCH ? ...) so
the MATCH clause only receives cleaned tokens and cannot be exploited by
injection.

In `@src/services/memory-lifecycle.ts`:
- Around line 176-208: The shard transaction started with db.run("BEGIN
TRANSACTION") must be rolled back on any exception to avoid leaving the DB
handle mid-transaction; update the catch block in the applyDecay
(memory-lifecycle) logic to call db.run("ROLLBACK") (or connection-level
rollback) before logging and rethrowing/returning, ensuring the COMMIT remains
in the happy path and that archiveMemory or updateStmt.run errors also trigger
the rollback path.
- Around line 178-189: The loop is re-applying total lifetime decay each run by
using age since creation against the already-decayed currentStrength; change it
to apply only incremental decay since the last maintenance pass: compute deltaMs
= now - (memory.last_decay_at || memory.created_at), use deltaDays = deltaMs /
(24*60*60*1000), then compute newStrength = currentStrength *
Math.exp(-decayRate * deltaDays), clamp and save, and set memory.last_decay_at =
now (or persist equivalent field); this prevents compounding full-life decay on
every run.

In `@src/services/memory-scoring-service.ts`:
- Around line 76-163: The shard loop starts a DB transaction with db.run("BEGIN
TRANSACTION") but on error only logs and continues, leaving the transaction open
and already incremented totalUpdated; fix by executing db.run("ROLLBACK") in the
catch path (or a finally that rolls back if not committed) and move the
increment of totalUpdated out of the per-memory loop into a per-shard success
branch (use a local shardUpdated counter that you increment after each
updateStmt.run and only add shardUpdated to totalUpdated after db.run("COMMIT")
succeeds). Ensure references: db.run("BEGIN TRANSACTION"), updateStmt.run,
db.run("COMMIT"), the catch block that logs the error, and the
totalUpdated/shardsProcessed variables are updated accordingly.

In `@src/services/migration-service.ts`:
- Around line 66-76: The code only triggers migration when storedDimensions
differs from CONFIG.embeddingDimensions, but storedModel is also read and should
trigger migration when it differs; update the conditional in the blocks around
the mismatches push (the one comparing storedDimensions and the similar block at
108-116) to check for either a dimensions mismatch OR a model mismatch (e.g.,
storedDimensions !== CONFIG.embeddingDimensions || storedModel !==
CONFIG.embeddingModel), keep collecting vectorCount via
vectorSearch.countAllVectors(db) and push the same mismatches object (including
shard.id and shard.dbPath) so model-only changes will correctly schedule a
migration.
- Around line 251-304: The code currently calls
shardManager.deleteShard(shardInfo.shardId) before re-embedding each memory;
change to a two-phase migration: do not delete the source shard up front—first
iterate tempMemories and for each memory use embeddingService.embedWithTimeout
and vectorSearch.insertVector into the target shard (via
shardManager.getWriteShard and connectionManager.getConnection), track
successful inserts (e.g., increment a local success counter instead of calling
shardManager.incrementVectorCount on the source), and optionally pin via
vectorSearch.pinMemory; after the loop verify the expected counts
(reEmbeddedCount or a newly tracked success count equals tempMemories.length or
meets your validation), only then call
shardManager.deleteShard(shardInfo.shardId) and update
shardManager.incrementVectorCount on the destination shards; keep existing error
logging for failed items and ensure processedCount/reportProgress still reflect
progress during phase one.

In `@src/services/platform-server.ts`:
- Around line 38-43: The request body is read into chunks with no size cap
(variables chunks, req, body) which allows memory-exhaustion DoS; add a
configurable MAX_BODY_SIZE (e.g. in bytes), track a running total while
iterating for await (const chunk of req), and if total exceeds MAX_BODY_SIZE
immediately stop reading, destroy/close the stream and return an appropriate 413
response or throw an error; ensure you free resources (e.g., req.destroy()) and
avoid concatenating Buffer.concat when the limit was exceeded so only bounded
memory is used.

In `@src/services/retrieval-context.ts`:
- Around line 70-77: Normalize and canonicalize project paths before comparing
to ensure platform-agnostic equality/prefix checks: import and use Node's path
utilities to compute normalized/resolved forms (e.g., const a =
path.normalize(path.resolve(context.projectPath)) and const b =
path.normalize(path.resolve(result.projectPath))), replace backslashes with
forward slashes or use path.sep consistently, and on Windows compare with
toLowerCase() to handle case-insensitivity; then use those normalized variables
in the existing comparisons (result.projectPath === context.projectPath,
startsWith checks) so score and boost logic remains correct.

In `@src/services/secret-resolver.ts`:
- Around line 24-27: The permission check using "mode > 0o600" is unreliable;
update the check in secret-resolver.ts (where filePath and mode are used) to use
a bitmask on the file mode to detect any group/other permissions (e.g. test
(mode & 0o077) !== 0) instead of numeric comparison, and adjust the warning
message to indicate which insecure bits were set (group/other) for clarity.

In `@src/services/sqlite/shard-manager.ts`:
- Around line 145-152: The INSERTs into shard_metadata are currently building
SQL with string interpolation for CONFIG.embeddingDimensions and
CONFIG.embeddingModel in shard-manager.ts; change these to use
parameterized/prepared statements (e.g., db.run("INSERT OR REPLACE INTO
shard_metadata (key, value) VALUES (?, ?)", [key, value]) or prepare/stmt.run)
when writing the 'embedding_dimensions' and 'embedding_model' rows so values are
passed as parameters (ensure embeddingDimensions is converted to a string) to
avoid SQL syntax issues and injection risk; update the calls that reference
these specific INSERTs accordingly.

In `@src/services/sqlite/transcript-manager.ts`:
- Around line 48-55: The transcripts table currently defines id as TEXT PRIMARY
KEY but the FTS5 external index uses content_rowid='id' and code pushes
new.id/old.id into rowid, which breaks FTS5 expectations (content_rowid must map
to an INTEGER PRIMARY KEY). Fix by making the transcripts table use an INTEGER
PRIMARY KEY (rowid-backed) for the FTS mapping (e.g., change the CREATE TABLE
transcripts id column to an INTEGER PRIMARY KEY AUTOINCREMENT or add a separate
INTEGER primary key column) and update any code paths that insert/return IDs so
they use the numeric rowid for the FTS operations; update the references around
content_rowid='id' and the places that push new.id/old.id into rowid to use the
numeric primary key instead (in this file: the CREATE TABLE transcripts
statement, the FTS create statements that use content_rowid='id', and the code
blocks that push new.id/old.id into rowid).

In `@src/services/sqlite/vector-search.ts`:
- Around line 271-289: The code is directly JSON.parse-ing row.metadata in
retrieval paths (used when computing contextBoost and when building the return
object), which will throw on corrupt JSON and break searches; update both
occurrences (the metadata parse passed into calculateContextBoost and the
metadata field in the returned object—also the similar spots around lines
568–572) to use a safe parse pattern: try/catch or a small helper (e.g.,
safeParseMetadata) that returns undefined on parse error and optionally logs a
warning including the row.id, then pass the safe result into
calculateContextBoost and set metadata to the safe result in the returned object
so a corrupt metadata string no longer breaks the query flow.
- Around line 559-567: The WHERE clause in getMemoriesBySessionID builds the
LIKE pattern by interpolating sessionID directly, so literal '%' or '_' in
sessionID act as SQL wildcards and over-match; fix it by escaping '%' and '_'
(and the escape char '\' itself) in the sessionID value, build the pattern as
'%' + escapedSessionID + '%', use a parameterized query (the prepared stmt
above) and add "ESCAPE '\'" to the SQL (or the DB's equivalent) so the escaped
characters are treated literally; update the call that sets the bound parameter
for stmt.all(...) to use the escaped pattern instead of string interpolation.
- Around line 86-97: In insertVector, when a partial backend.insert succeeds
(e.g., content inserted but tags insert fails) we must compensate by removing
any backend index entries that succeeded before deleting the DB row; modify the
try/catch to track which kinds were inserted (use getBackend() and record
successes for "content" and "tags"), and in the catch block call backend.delete
or the appropriate removal API for each successful kind (e.g., backend.delete({
id: record.id, shard, kind })) before running db.prepare(...).run(record.id);
make the cleanup best-effort (catch/log but continue) so the DB row removal
still happens and avoid leaving stale backend entries.

In `@src/services/transcript-capture.ts`:
- Around line 23-24: The global boolean isCaptureRunning is causing captures to
be skipped across sessions; replace it with a per-session lock (e.g., a Map or
Set keyed by sessionId/connectionId) and use that instead of isCaptureRunning in
the capture entry/exit paths: check lock.has(sessionId) and return if true, then
lock.set(sessionId, true) when starting and lock.delete(sessionId) when finished
or on error; update both places where isCaptureRunning is referenced (the
initial check/set shown and the other occurrence around lines 78-79) and ensure
all start/stop/error paths clear the per-session lock to avoid leaking locks.

In `@src/services/user-memory-learning.ts`:
- Around line 33-35: The code uses a shared literal "unknown" when
tags.user.userEmail is missing which risks merging different users into one
profile; instead derive a safe unique fallback (e.g., use tags.user.userId or
tags.sessionId if available, or generate a per-request UUID) and pass that to
userProfileManager.getActiveProfile; update the same pattern found around the
block referencing userId and the later usage at lines 58-63 so every call to
getActiveProfile or any profile lookup uses a non-colliding identifier rather
than the fixed "unknown" string.
- Around line 214-218: The log call in user-memory-learning.ts is leaking a
secret by including CONFIG.memoryApiKey; remove memoryApiKey from the logged
object (or replace it with a masked/boolean indicator like hasMemoryApiKey) in
the log("User Profile Config Check Failed:", ...) invocation so only
non-sensitive fields (e.g., memoryModel, memoryApiUrl) are emitted; specifically
update the log call that references CONFIG.memoryModel, CONFIG.memoryApiUrl,
CONFIG.memoryApiKey to exclude or mask CONFIG.memoryApiKey.

In `@src/services/user-profile/profile-context.ts`:
- Around line 11-37: The code currently calls JSON.parse(profile.profileData)
and immediately dereferences profileData.preferences, patterns, and workflows
which can throw or be undefined for malformed persisted data; wrap the
JSON.parse in a try/catch and validate the resulting object shape, ensuring each
of preferences, patterns, workflows is an Array (use Array.isArray) and default
them to [] if missing or invalid, then proceed with sort/slice/forEach; update
references to profileData in this file (e.g., variable profileData and the
blocks that access profileData.preferences/patterns/workflows) to use the
validated/defaulted arrays so context generation never throws on bad JSON.

In `@src/services/user-profile/profile-utils.ts`:
- Around line 21-23: The current flattening branch in safeArray (the else if
(item) { flattened.push(item); } block) drops valid falsy values like 0 and
false; change the check to explicitly allow non-null/undefined values (e.g.,
item !== undefined && item !== null) so only null/undefined are skipped, keeping
references to safeArray, item, and flattened to locate the change.

In `@src/services/user-profile/user-profile-manager.ts`:
- Around line 145-169: The current profile update flow (getVersionStmt,
updateStmt, addChangelog, cleanupOldChangelogs) performs SELECT, UPDATE and
changelog INSERT as separate operations and can race; wrap the read-version,
profile UPDATE and changelog insertion in a single SQL transaction (e.g., BEGIN
IMMEDIATE ... COMMIT or use your DB library’s transaction API) so obtaining
version, applying update (updateStmt) and calling addChangelog(profileId,
newVersion, ...) are executed atomically; ensure the transaction increments the
version based on the selected value, runs the UPDATE with the newVersion and
performs the changelog INSERT before committing, and roll back on any error.

In `@src/services/vector-backends/backend-factory.ts`:
- Around line 22-32: The insert, insertBatch, and delete methods in
backend-factory.ts currently call this.activeBackend.* directly and therefore
skip the degradation/fallback logic; modify these methods to follow the same
fallback pattern used for reads: invoke the shared degradation wrapper (or
implement the same try/catch + this.degradeToExactScan() behavior) so that on
backend errors the factory flips to the exact-scan fallback and retries or
routes the write to the fallback backend; update the methods named insert,
insertBatch, and delete (and any internal helper used for reads such as
degradeIfNeeded or degradeToExactScan) to catch backend failures, call the
degradation routine, and then retry or forward the operation to the fallback
backend.

In `@src/services/vector-backends/usearch-backend.ts`:
- Around line 173-179: getOrCreateIndex has a race: two concurrent calls for the
same indexKey can both await createEmptyIndex and then one set will overwrite
the other losing inserted vectors; fix by deduping in-flight initializations in
getOrCreateIndex (e.g., store a Promise<CachedIndex> in a separate inFlight map
or store the promise in this.indexes before awaiting createEmptyIndex), await
the stored promise for subsequent callers, and ensure only the first successful
creation writes the final CachedIndex (or replace promise with resolved value)
so concurrent callers share the same initialization instead of creating
duplicate caches.

In `@src/services/web-server-worker.ts`:
- Around line 49-263: The worker's handleRequest currently skips auth and PII
redaction; add the same x-opencode-mem-key check and response-redaction used in
WebServer: call the shared validateMemKey(req.headers.get("x-opencode-mem-key"))
at the top of mutating/ sensitive routes (e.g., handleAddMemory,
handleUpdateMemory, handleDeleteMemory, handleBulkDelete, handlePinMemory,
handleUnpinMemory, handleRefreshProfile, handleGetUserProfile,
handleGetProfileSnapshot, handleGetProfileChangelog) and return a 401 JSON when
invalid; after receiving results from profile/memory handlers
(handleGetUserProfile, handleListMemories, handleGetProfileSnapshot,
handleGetProfileChangelog, etc.) pass them through the same
redactSensitiveFields/redactMemoryFields helper used by WebServer before calling
jsonResponse so PII is stripped. Ensure you reference and reuse the existing
helpers (validateMemKey, redactSensitiveFields/redactMemoryFields, jsonResponse)
rather than duplicating logic.

In `@src/services/web-server.ts`:
- Around line 61-68: The startPromise is never cleared on shutdown so start()
returns a stale resolved promise; modify the stop() implementation (the
WebServer.stop method) to clear/reset this.startPromise (e.g., set
this.startPromise = undefined/null) after the server has fully stopped (and
after any awaited shutdown work) so subsequent calls to start() will call
_start() again; ensure you also handle any error paths in stop() so startPromise
is always cleared.
- Around line 449-454: The GET handler in web-server.ts is reading the wrong
query parameter name ("chlogId"), causing valid requests using "changelogId" to
be rejected; update the query lookup to use "changelogId" (and rename the local
variable if desired) before calling handleGetProfileSnapshot(changelogId) so the
parameter matches the API contract and the existing error message.

In `@src/web/app.js`:
- Around line 32-46: The fetchAPI function currently clears timeoutId only on
the happy path, so move the clearTimeout(timeoutId) into a finally block to
ensure the timeout is always cleared (and the AbortController signal cleaned up)
even if fetch or response.json() throws; update fetchAPI to create controller
and timeoutId as now, await fetch/response.json() in try/catch, and perform
clearTimeout(timeoutId) (and any controller cleanup if needed) inside a finally
so timeoutId is always cleared.
- Around line 157-160: Replace inline onclick HTML with safe DOM creation and
event binding: stop interpolating memory.id into onclick strings in the
pinButton construction (and the other occurrences mentioned), instead render a
button with a data-memory-id attribute (e.g., data-memory-id="${memory.id}") and
a stable class (e.g., btn-pin / pinned) and then in the client script locate
those buttons (querySelectorAll by class or data attribute) and call
addEventListener to invoke the existing pinMemory/unpinMemory functions passing
the dataset id. Ensure any place that currently constructs HTML with onclick
(references: the pinButton variable and similar snippets at the other reported
ranges) is changed to use data-* attributes plus event listeners so IDs are not
injected into HTML attributes.
- Around line 509-531: The unconditional showToast call causes success messages
even when deletes fail; update the bulk-delete flow (the block using promptIds,
memoryIds and fetchAPI) to aggregate API outcomes and deletedCount, check each
result.success and result.data.deleted, and only call
showToast(t("toast-bulk-delete-success"), "success") when deletedCount equals
the total requested ids; otherwise show a failure/partial message indicating how
many deletions succeeded vs requested (use t keys for localization) and avoid
clearing state.selectedMemories or calling
loadMemories/loadStats/updateBulkActions until you decide whether to refresh;
ensure you still handle partial success by updating selectedMemories
appropriately and surface API errors when result.success is false.

In `@src/web/i18n.js`:
- Around line 225-227: The t function assumes translations[lang] exists and will
throw if a persisted locale is invalid; update t (the function named t and its
use of translations and getLanguage) to guard the language lookup by falling
back to translations["en"] (or an empty object) when translations[lang] is
falsy/undefined before accessing [key], and ensure getLanguage's result is
normalized (e.g., default to "en") so the lookup becomes safe and returns
translations[lang][key] || translations["en"][key] || key without causing a
runtime crash.

In `@tests/language-detector.test.ts`:
- Around line 9-11: The test case for the "short Chinese (5+ chars)" boundary is
using the long sample from the earlier test; update the test to actually
exercise the 5-character boundary by replacing the input string in the it block
that calls detectLanguage("这是一个中文测试") with a short 5-character Chinese string
(e.g., "测试中文五") and ensure the it description still states "should detect short
Chinese as zh (5+ chars)"; keep the assertion expect(detectLanguage(<new 5-char
string>)).toBe("zh") so the detectLanguage function is validated at the
boundary.

In `@tests/memory-engine.test.ts`:
- Around line 615-656: The test sets a local flag conflictSaved inside the
db.prepare mock when the "memory_conflicts" branch runs but never asserts it;
update the test that calls detectConflicts (the invocation using "new-1", "Use
TypeScript..." and "mem_project_abc") to assert that conflictSaved is true after
the call so the test fails if the persistence path in the mock is not executed
(i.e., add an assertion like expect(conflictSaved).toBe(true) immediately after
obtaining conflicts); reference the existing conflictSaved variable, the
db.prepare mock branch for "memory_conflicts", and the detectConflicts call to
locate where to add the assertion.
- Around line 376-387: Wrap the test's temporary mutation of
CONFIG.transcriptStorage.enabled in a try/finally to guarantee restoration even
if the test throws: in the test that disables transcripts, assign original =
CONFIG.transcriptStorage.enabled, set CONFIG.transcriptStorage.enabled = false,
then run the code that constructs new TranscriptManager() and calls
saveTranscript("sess", "/p", []), and in a finally block restore
CONFIG.transcriptStorage.enabled = original; reference CONFIG,
TranscriptManager, and saveTranscript in your change so the mutation and restore
are obvious.

In `@tests/profile-tool-runtime.test.ts`:
- Around line 85-89: The try/catch around the cleanup DB calls (db.run("DELETE
FROM user_profile_changelogs") and db.run("DELETE FROM user_profiles"))
currently swallows all errors; replace the empty catch with handling that
surfaces failures (e.g., catch (err) and log the error with context and rethrow
or fail the test) so locked DBs or other cleanup issues aren’t hidden during
test runs.

In `@tests/tags.test.ts`:
- Around line 22-26: The test for getProjectName currently only checks the
return type; change it to assert the exact expected project name for the
trailing-slash case. In the it("should handle trailing separator") test replace
the typeof assertion with an equality check such as
expect(result).toBe("my-app") so the test fails on incorrect parsing by
getProjectName.

In `@tests/vector-backends/exact-scan-backend.test.ts`:
- Around line 40-103: The tests leave SQLite Database handles open causing
temp-dir cleanup to fail on some platforms; after each test that creates a
Database (the db variable in tests "searches vectors from sqlite blobs in
similarity order" and "returns empty search result when sqlite has no vectors"),
close the handle by calling db.close() (or ensure Database#close is invoked)
once you're done using it (ideally just after the backend.search call or in a
finally/afterEach block) so the file locks are released; apply the same closure
pattern for both Database instances referenced alongside ExactScanBackend and
shard in these tests.

In `@tests/vector-backends/usearch-backend.test.ts`:
- Around line 115-149: The test "rebuilds an index from sqlite rows" opens a
SQLite Database instance named db but never closes it, which can leave test.db
locked; ensure the Database (db) is closed when the test finishes — e.g., call
db.close() (or move db into a try/finally that closes it, or add an afterEach
that closes the Database) after the assertions in the test that uses
USearchBackend.rebuildFromShard and search so the temp dir cleanup won't fail on
Windows.

---

Nitpick comments:
In `@src/services/ai/tools/tool-schema.ts`:
- Around line 8-9: The schema's properties field currently uses Record<string,
any> (appearing on the properties declarations around lines with properties:
Record<string, any>), which weakens type safety; change those occurrences to a
safer type such as Record<string, unknown> or introduce a generic type parameter
(e.g., ToolSchema<T = unknown> or a named SchemaProperties = Record<string,
unknown>) and apply it to the properties declarations so callers can narrow
payload shapes; update the interface/type declarations that reference properties
(and any functions or classes using ToolSchema) to use the new generic or
SchemaProperties alias to preserve compatibility and stronger compile-time
checks.

In `@src/services/api-handlers.ts`:
- Around line 511-536: The current loop does repeated shard scans by iterating
uniqueTags and calling shardManager.getAllShards + vectorSearch.searchInShard
for each tag; instead build a mapping from shard (use shard.id or shard.dbPath
from projectShards/shardManager.getAllShards) to the list of container tags that
belong to that shard (use extractScopeFromTag to determine scope/hash), then
iterate each shard once and perform a single search per shard (modify or
overload vectorSearch.searchInShard to accept multiple container tags or run one
search and filter by tags in-memory) and push results into memoryResults; update
error handling around the single shard search accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c3a4c52f-f932-40b2-9cfd-2c376ca92b08

📥 Commits

Reviewing files that changed from the base of the PR and between 00d96de and cc5f354.

📒 Files selected for processing (83)
  • scripts/build.mjs
  • scripts/migrate-tests.mjs
  • scripts/migrate-v1-to-v2.ts
  • src/config.ts
  • src/index.ts
  • src/plugin.ts
  • src/services/ai/ai-provider-factory.ts
  • src/services/ai/opencode-provider.ts
  • src/services/ai/provider-config.ts
  • src/services/ai/providers/anthropic-messages.ts
  • src/services/ai/providers/base-provider.ts
  • src/services/ai/providers/google-gemini.ts
  • src/services/ai/providers/openai-chat-completion.ts
  • src/services/ai/providers/openai-responses.ts
  • src/services/ai/session/ai-session-manager.ts
  • src/services/ai/session/session-types.ts
  • src/services/ai/tools/tool-schema.ts
  • src/services/ai/validators/user-profile-validator.ts
  • src/services/api-handlers.ts
  • src/services/auto-capture.ts
  • src/services/cleanup-service.ts
  • src/services/client.ts
  • src/services/context.ts
  • src/services/deduplication-service.ts
  • src/services/embedding.ts
  • src/services/jsonc.ts
  • src/services/language-detector.ts
  • src/services/logger.ts
  • src/services/memory-conflicts.ts
  • src/services/memory-lifecycle.ts
  • src/services/memory-scoring-service.ts
  • src/services/memory-scoring.ts
  • src/services/migration-service.ts
  • src/services/platform-server.ts
  • src/services/privacy.ts
  • src/services/retrieval-context.ts
  • src/services/secret-resolver.ts
  • src/services/sqlite/connection-manager.ts
  • src/services/sqlite/shard-manager.ts
  • src/services/sqlite/sqlite-bootstrap.ts
  • src/services/sqlite/transcript-manager.ts
  • src/services/sqlite/types.ts
  • src/services/sqlite/vector-search.ts
  • src/services/tags.ts
  • src/services/transcript-capture.ts
  • src/services/user-memory-learning.ts
  • src/services/user-profile/profile-context.ts
  • src/services/user-profile/profile-utils.ts
  • src/services/user-profile/types.ts
  • src/services/user-profile/user-profile-manager.ts
  • src/services/user-prompt/user-prompt-manager.ts
  • src/services/vector-backends/backend-factory.ts
  • src/services/vector-backends/exact-scan-backend.ts
  • src/services/vector-backends/types.ts
  • src/services/vector-backends/usearch-backend.ts
  • src/services/web-server-worker.ts
  • src/services/web-server.ts
  • src/types/index.ts
  • src/types/usearch.d.ts
  • src/web/app.js
  • src/web/i18n.js
  • tests/ai-provider-config.test.ts
  • tests/anthropic-provider.test.ts
  • tests/config-resolution.test.ts
  • tests/config.test.ts
  • tests/language-detector.test.ts
  • tests/memory-engine.test.ts
  • tests/memory-scope.test.ts
  • tests/openai-chat-completion-provider.test.ts
  • tests/opencode-provider.test.ts
  • tests/plugin-loader-contract.test.ts
  • tests/privacy.test.ts
  • tests/profile-tool-runtime.test.ts
  • tests/profile-write.test.ts
  • tests/project-scope.test.ts
  • tests/tags.test.ts
  • tests/tool-scope.test.ts
  • tests/vector-backends/backend-factory.test.ts
  • tests/vector-backends/exact-scan-backend.test.ts
  • tests/vector-backends/migration-fallback.test.ts
  • tests/vector-backends/usearch-backend.test.ts
  • tests/vector-search-backend-integration.test.ts
  • tests/windows-path.test.ts

@ZeR020 ZeR020 closed this May 6, 2026
@ZeR020
ZeR020 deleted the audit-full-repo-3 branch May 6, 2026 19:56
@ZeR020 ZeR020 changed the title 🔍 Round 3 — Full Codebase Audit fix: apply follow-up repository audit fixes May 6, 2026
@ZeR020 ZeR020 changed the title fix: apply follow-up repository audit fixes fix: apply follow-up quality fixes May 6, 2026
ZeR020 added a commit that referenced this pull request Jul 19, 2026
GitHub CodeQL flagged localStorage.setItem of the dashboard API key as
js/clear-text-storage-of-sensitive-data (CWE-312/315). The alert is
technically accurate on the pattern but the implied risk is not realizable
under the current threat model, so alert #8 was dismissed as false positive.

Added an inline security comment on saveSettings() recording the rationale
so future reviewers/auditors see it with the code:

- Dashboard binds to 127.0.0.1 (config.ts webServerHost) — no remote
  attacker can reach it without an explicit host override.
- Only XSS on the dashboard can read localStorage; such XSS already has
  same-origin access to everything the key protects (the user's memories).
- Server-side CSP enforced (19511cd, f51b0ac); user content escaped.

Includes a revisit trigger: if the dashboard is ever hosted non-loopback,
move to sessionStorage with short TTL or exchange the raw key for a
non-extractable comparison-only token.

No behavior change — comment only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant